Skip to content

perf(plugins): run scheduled updates off the render thread#407

Open
ChuckBuilds wants to merge 1 commit into
mainfrom
perf/async-plugin-updates
Open

perf(plugins): run scheduled updates off the render thread#407
ChuckBuilds wants to merge 1 commit into
mainfrom
perf/async-plugin-updates

Conversation

@ChuckBuilds

Copy link
Copy Markdown
Owner

Summary

PR 5 of the performance series — the biggest UX defect found in the deep-dive: plugin update() ran inline in the render loop (execute_update's thread.join(timeout=30) blocked it), so one slow HTTP fetch froze scrolling for the whole fetch. On a flaky network this is constant: we watched soccer's ESPN DNS-retry storms stall the devpi repeatedly.

Design (safety-first):

  • Scheduling logic untouched — every existing gate stays on the render thread (enabled, circuit breaker, can_execute, interval). Only execution moves, to a single serialized worker (same one-at-a-time behavior as before; no concurrent fetch herd). RUNNING set at enqueue + a pending-set gives double re-entry protection.
  • Per-plugin locks make the old implicit update/display no-overlap guarantee explicit — and strengthen it: previously, after a timeout, the lingering update thread overlapped display(); now the lock holds through that window too. The display side try-locks; if the plugin is mid-update it holds the last frame for that iteration and reports success, so a skip never falsely advances rotation. Deadlock-free by construction (worker holds one lock; display never blocks).
  • Timeout semantics byte-identical (unkillable lingering thread documented in-code).
  • Kill switch: plugin_system.synchronous_updates: true restores the inline path with no deploy.
  • Thread-safety audit: state_manager is RLock-guarded; health_tracker uses whole-value per-key writes (worst case one-cycle staleness, self-correcting — no worse than today's post-timeout overlap).

Verification

  • 8 concurrency tests: scheduler returns <100ms with a 2s update in flight; overlap-assertion under a 500Hz try-lock display loop across repeated updates (zero violations); lock released on failure paths; no double-enqueue; unload-while-queued harmless; kill switch blocks inline as before.
  • Existing suites green (one pre-existing failure, test_circuit_breaker, is the mock drift fixed in fix(core): harden text-measurement caches; surface snapshot failures #400).
  • Devpi soak: updates completing on the worker, rotation advancing, zero stuck RUNNING states, no new errors. Extended soak continues on the devpi.

🤖 Generated with Claude Code

https://claude.ai/code/session_01FqzC1nzTWL4kaqgMaQZFam

plugin update() executed inline in the render loop — execute_update's
internal thread.join(timeout=30) blocked it, so one slow plugin HTTP
fetch froze scrolling for the whole fetch (up to 30s; DNS-retry storms
made this a regular occurrence on flaky networks).

Scheduling stays on the render thread and keeps every existing gate
(enabled, circuit breaker, can_execute, interval); due updates are now
enqueued to a single background worker (serialized — same one-at-a-time
execution as before, no thundering herd). RUNNING is set at enqueue so
can_execute blocks re-entry alongside the pending-set dedup.

Per-plugin locks make the old implicit update/display no-overlap
guarantee explicit: the worker holds the plugin's lock through its
update; the display side try-locks and, when the plugin is mid-update,
holds the last frame for that iteration — reported as success so a
mid-update skip never advances the rotation. Unlike before, the
guarantee now also holds across the post-timeout window (previously the
lingering update thread overlapped display()). Deadlock-free by
construction: the worker takes one lock; display never blocks.

Timeout semantics unchanged (lingering daemon thread documented).
Kill switch: plugin_system.synchronous_updates: true restores the
inline path.

8 new concurrency tests (non-blocking scheduler, overlap assertion
under a hammering display loop, lock release on failure/timeout paths,
dedup, kill switch); 4-min devpi soak clean (updates completing,
rotation advancing, no stuck RUNNING states).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FqzC1nzTWL4kaqgMaQZFam
@coderabbitai

coderabbitai Bot commented Jul 12, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@ChuckBuilds, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 43 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: afe5404f-5777-4b40-a239-172a76bfa07a

📥 Commits

Reviewing files that changed from the base of the PR and between 6edd80d and c32aaf0.

📒 Files selected for processing (4)
  • src/display_controller.py
  • src/plugin_system/plugin_manager.py
  • test/test_async_plugin_updates.py
  • test/test_plugin_system.py
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch perf/async-plugin-updates

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codacy-production

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 28 complexity · 2 duplication

Metric Results
Complexity 28
Duplication 2

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant